gRPC: Add gRPC interface to cardano-node with tests in cardano-testnet#6273
gRPC: Add gRPC interface to cardano-node with tests in cardano-testnet#6273carbolymer wants to merge 7 commits intomasterfrom
Conversation
864cf20 to
12bae80
Compare
0004d02 to
2c89086
Compare
aaab28c to
eab2ed1
Compare
4d19d4c to
98d8073
Compare
12d7ab5 to
73c37e6
Compare
8cbf3f2 to
0ec9ea7
Compare
7c29f29 to
ebf5f8b
Compare
a8564a9 to
5d772e5
Compare
5d772e5 to
450a849
Compare
86ec635 to
9984a35
Compare
7b30930 to
8e67434
Compare
2ea442d to
8f6a3b7
Compare
Jimbo4350
left a comment
There was a problem hiding this comment.
Minor comments but I would solicit feedback from @jasagredo regarding the configuration change. Will approve after that is discussed.
| , pncResponderCoreAffinityPolicy :: !(Last ResponderCoreAffinityPolicy) | ||
|
|
||
| -- gRPC | ||
| , pncRpcConfig :: !PartialRpcConfig |
There was a problem hiding this comment.
Same here regarding the configuration being moved to consensus.
| , ncResponderCoreAffinityPolicy :: ResponderCoreAffinityPolicy | ||
|
|
||
| -- gRPC | ||
| , ncRpcConfig :: RpcConfig |
There was a problem hiding this comment.
Hmm. @jasagredo is working on moving the cardano node configuration machinery to ouroboros-consensus. To prevent future pain I would run this change by him.
cardano-testnet/test/cardano-testnet-golden/files/golden/help/cardano.cli
Show resolved
Hide resolved
| mconcat $ | ||
| ("reason" .= prettyShow tr) | ||
| : case tr of | ||
| TraceRpcFatalError _ -> ["kind" .= String "FatalError"] |
There was a problem hiding this comment.
Where can one see the reason for the fatal error?
There was a problem hiding this comment.
It's above in line 29. I've made reason dumped for each trace.
In general, this is used for any initialisation exception: https://github.com/IntersectMBO/cardano-api/blob/2aa5ce77b1139d1d216c19eb4eb7b7a7e0e4e18b/cardano-rpc/src/Cardano/Rpc/Server.hs#L71
| --------------------------- | ||
| -- Test readParams response | ||
| --------------------------- | ||
| pparamsResponse ^. U5c.ledgerTip . U5c.slot === slot |
There was a problem hiding this comment.
How flaky is this test?
There was a problem hiding this comment.
I have not observed flakiness. This test also runs with 2 retries, so if this happens, it should rerun. We could call cli and get the tip in parallel, but I'm not sure the gain here will be worth of complexity.
cardano-testnet/test/cardano-testnet-test/Cardano/Testnet/Test/Rpc/Query.hs
Show resolved
Hide resolved
cardano-testnet/test/cardano-testnet-test/Cardano/Testnet/Test/Rpc/Transaction.hs
Show resolved
Hide resolved
|
|
||
| txBody <- H.leftFail $ createTransactionBody sbe content | ||
|
|
||
| let signedTx = signShelleyTransaction sbe txBody [wit0] |
There was a problem hiding this comment.
I see we are using the "old" api. Fine for this PR but I'd like you to migrate to the new api if possible in a follow up PR.
cardano-testnet/test/cardano-testnet-test/Cardano/Testnet/Test/Rpc/Transaction.hs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
nix/pkgs.nix:1
previousAttrs.buildInputsis not guaranteed to exist on all derivations; if it's missing, Nix evaluation will fail. Safer pattern is to append to(previousAttrs.buildInputs or [])before addingprev.windows.pthreads.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
This PR adds an experimental gRPC endpoint to
cardano-node. You can read architecture design here. For now the following methods are supported:The gRPC endpoint is disabled by default, can be enabled by CLI flag or by configuration option.
Support for enabling gRPC endpoint is added into cardano-testnet.
Things necessary to do before the merge:
cardano-clito remove SRPcardano-rpcto remove SRPChecklist
See Runnings tests for more details
CHANGELOG.mdfor affected package.cabalfiles are updatedhlint. See.github/workflows/check-hlint.ymlto get thehlintversionstylish-haskell. See.github/workflows/stylish-haskell.ymlto get thestylish-haskellversionghc-9.6andghc-9.12Note on CI
If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.